Skip to content

Conversation

@RohanNankani
Copy link
Collaborator

@RohanNankani RohanNankani commented Mar 11, 2025

Skipping this for quick merge

Mayank808 and others added 5 commits February 10, 2025 20:05
PR aims to fix the issue we are having with the ruff linting check
during our CI.
## Notion ticket link
<!-- Please replace with your ticket's URL -->
[Add startup Logs (Fix logging) for
backend](https://www.notion.so/uwblueprintexecs/Startup-logs-14e10f3fb1dc8015a1c5c65e9dcd2c55?pvs=4)


<!-- Give a quick summary of the implementation details, provide design
justifications if necessary -->
## Implementation description
* We were having issues troubleshooting where some people may have
startup issues, since errors weren't being emitted to the console
(through standard error). After deep diving, we realized that the
`alembic.ini` config was taking control over logging configurations and
suppressing other loggers. Given that going without the `alembic.ini`
config doesn't seem viable at the moment, I've stuck to basing all
loggers in the API off of one sublogger, and using a function to create
child loggers for more context.


<!-- What should the reviewer do to verify your changes? Describe
expected results and include screenshots when appropriate -->
## Steps to test
1. Run `pdm run dev`
2. Add a new logger using the instructions in the README to another
service, ie.
To add a logger to a new service or file, use the `LOGGER_NAME` function
in `app/utilities/constants.py`

```python
from app.utilities.constants import LOGGER_NAME

log = logging.getLogger(LOGGER_NAME("my_service"))
```

<!-- Draw attention to the substantial parts of your PR or anything
you'd like a second opinion on -->
## What should reviewers focus on?
* Whether logging works


## Checklist
- [x] My PR name is descriptive and in imperative tense
- [x] My commit messages are descriptive and in imperative tense. My
commits are atomic and trivial commits are squashed or fixup'd into
non-trivial commits
- [x] I have run the appropriate linter(s)
- [x] I have requested a review from the PL, as well as other devs who
have background knowledge on this PR or who will be building on top of
this PR

---------

Co-authored-by: Evan Wu <[email protected]>
## Notion ticket link
<!-- Please replace with your ticket's URL -->
[Setup Email
System](https://www.notion.so/uwblueprintexecs/Setup-Basic-Email-System-11110f3fb1dc80a48e88ce63228b926c?pvs=4)

NOTE THIS IS A PR for feedback not to merge. 
Will be removing email.py route as its simply there right now for
testing
will be removing todos and adding comments 
will be removing any prints or updating them to logs

<!-- Give a quick summary of the implementation details, provide design
justifications if necessary -->
## Implementation description
* Implemented Email sending using amazon ses.
* Create email templates at startup
* use email templates to send single and bulk emails to users with
predefined templates
* Email templates can use inline CSS need to define a text file with
only text for each respective email

Process to create a new Email Template to use:

1. Create the HTML and Text files for the new template. Use inline CSS
and direct hosted links to any images required.
2. Update `backend/app/utilities/ses/ses_templates.json` to include
template name subject and absolute paths to the above file
3. Create a new data class Type for the Email template
```python
@DataClass
class NewEmailTemplatData():
    name: str
    date: str
    random: int
 ```
4. Update Email template enum to include the file name of the template defined in amazon ses and the `ses_templates.json` file

```python
class EmailTemplate(Enum):
   NEWEMAILTEMPLATE = "newemailtemplate"
```

5. Use in code to send emails using email service
```python
email_service.send_email(
    template=EmailTemplate.NEWEMAILTEMPLATE,
    content=EmailContent[NewEmailTemplatData](
        recipient=recipient, 
data=NewEmailTemplatData(name=user_name, date="2021-12-01", random: 20)
    ),
)
```

## Todo
- [ ] Add bulk send email after general flow is good
- [ ] Look into schedule send
- [ ] Clean up logging and comments
- [ ] Remove email.py route
- [ ] Unit Testing
- [ ] File clean up


<!-- What should the reviewer do to verify your changes? Describe expected results and include screenshots when appropriate -->
## Steps to test
1. Simply hit the `/email/send-test` email route when running the be on local

<!-- Draw attention to the substantial parts of your PR or anything you'd like a second opinion on -->
## What should reviewers focus on?
* Focus on overall design of things and how things are laid out as this pr still needs to be cleaned up. 
* How templates are defined, what folders code blocks should be stored in
* Are we okay with running the email template check on startup
* Will be removing email.py route as its simply there right now for testing


## Checklist
- [x] My PR name is descriptive and in imperative tense
- [x] My commit messages are descriptive and in imperative tense. My commits are atomic and trivial commits are squashed or fixup'd into non-trivial commits
- [x] I have run the appropriate linter(s)
- [ ] I have requested a review from the PL, as well as other devs who have background knowledge on this PR or who will be building on top of this PR
## Notion ticket link
<!-- Please replace with your ticket's URL -->

[LLSC-30](https://www.notion.so/uwblueprintexecs/Task-Board-db95cd7b93f245f78ee85e3a8a6a316d)


<!-- Give a quick summary of the implementation details, provide design
justifications if necessary -->
## Implementation description
* 


<!-- What should the reviewer do to verify your changes? Describe
expected results and include screenshots when appropriate -->
## Steps to test
1.


<!-- Draw attention to the substantial parts of your PR or anything
you'd like a second opinion on -->
## What should reviewers focus on?
* 


## Checklist
- [ ] My PR name is descriptive and in imperative tense
- [ ] My commit messages are descriptive and in imperative tense. My
commits are atomic and trivial commits are squashed or fixup'd into
non-trivial commits
- [ ] I have run the appropriate linter(s)
- [ ] I have requested a review from the PL, as well as other devs who
have background knowledge on this PR or who will be building on top of
this PR
Copy link
Collaborator

@EdmondLi1 EdmondLi1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@RohanNankani RohanNankani changed the base branch from main to alex-branch March 11, 2025 01:02
@sunbagel sunbagel deleted the rohan-edmond/matches-model branch March 12, 2025 00:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants